fix(analytics): redact single-use path tokens before auto-capture (#3932) - #3935
Conversation
|
Warning Review limit reached
Next review available in: 59 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (5)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #3935 +/- ##
==========================================
+ Coverage 92.67% 92.69% +0.01%
==========================================
Files 169 169
Lines 5546 5556 +10
Branches 1782 1786 +4
==========================================
+ Hits 5140 5150 +10
Misses 326 326
Partials 80 80
Flags with carried forward coverage won't be shown. Click here to find out more. Continue to review full report in Codecov by Harness.
🚀 New features to boost your workflow:
|
…xtensions (#3961) * refactor(log): source sensitive redaction markers from config lib/ must stay module-agnostic — SENSITIVE_PATH_MARKERS and SENSITIVE_QUERY_KEYS in lib/helpers/redactUrl.js hardcoded auth/invitations route vocabulary owned by feature modules (#3935). Move both lists to config.log.sensitivePathMarkers / config.log.sensitiveQueryKeys (current values as defaults in config/defaults/development.config.js) so a module extends redaction coverage from its own config without editing shared lib/. redactUrl.js reads the lists from config once at module load, falling back to the built-in literals when config or the key is absent — redaction degrades safely instead of silently redacting nothing in an edge context. Exports/API unchanged (existing consumers: lib/middlewares/analytics.js, lib/services/express.js). Closes #3953 Claude-Session: https://claude.ai/code/session_01WfNC8bt1TgL4AsiYgCEGup * fix(log): union config-provided redaction lists with built-in defaults deepMerge replaces arrays (no union), so a module-level config.log.sensitivePathMarkers/sensitiveQueryKeys extension clobbered the global default instead of extending it, and a `??` fallback let an explicit empty array silently disable redaction for that vector. Compute the effective lists as a Set-deduped union of the built-in DEFAULT_SENSITIVE_* base with config.log.*, so config is purely additive from any layer and redaction can never be disabled via config. development.config.js's entries become empty arrays (the extension point), killing the literal duplication with redactUrl.js. Claude-Session: https://claude.ai/code/session_01WfNC8bt1TgL4AsiYgCEGup * fix(log): make redaction config extension layer-proof and shape-safe Phase-0 iteration-2 review findings on #3953: - Remove log.sensitivePathMarkers/sensitiveQueryKeys from config/defaults/development.config.js. deepMerge (config/index.js) replaces arrays wholesale, so declaring these as [] at Layer 2 (global defaults) silently clobbered any Layer 1 (module config) extension of the same key. Omitting the key lets a module value pass through untouched; a comment documents the extension point. - Guard redactUrl.js against non-array config values via a sanitizeConfigList helper: an object/number would throw at module-eval (boot crash), a string would spread char-by-char (silent over-redaction). Malformed values now fall back to [] (defaults still apply) and log a console.warn identifying the offending config path. - Add malformed-value tests (object + string) and a lockstep guard test pinning that development.config.js never re-declares either key. Claude-Session: https://claude.ai/code/session_01WfNC8bt1TgL4AsiYgCEGup * test(log): restore console.warn spy in finally to prevent mock leakage Claude-Session: https://claude.ai/code/session_01WfNC8bt1TgL4AsiYgCEGup
Problem
Analytics auto-capture recorded
endpoint = req.originalUrl.split('?')[0]— stripped the query string but not path params. Routes carrying a single-use secret as a path param (/api/auth/reset/:token,/verify-email/:token,/(auth/)invitations/verify/:token) shipped the raw token to the analytics store, enabling account takeover by anyone with analytics read access.Fix
New
redactPathSecretshelper (co-located with the existing morganredactUrl). The middleware now prefers the matched route pattern (req.route?.pathwith mounted prefix) and falls back toredactPathSecretsfor unmatched paths — so a token is never captured raw.Note (behavior change)
Preferring the route pattern applies to all parameterized routes, not only the token ones:
endpointnow reports the pattern (/api/tasks/:id) rather than the concrete path for matched routes. This lowers event cardinality (safer) but changes theendpointproperty shape for any existing analytics insight keyed on literal paths.Test
Added path-token redaction unit tests + a real
/api/auth/reset/:tokenintegration test; the existing query-string-scrub test still passes.Closes #3932
https://claude.ai/code/session_01V1mhCDsSUnn3WGtevd3YFX